feat(api)!: geef aanleveren een eigen resource zodat de generator-update kan landen - #203
Conversation
Upstream melden? Besluit gevraagdBij het uitzoeken van deze wijziging bleek de shadow-detectie in de generator niet te doen wat hij belooft. Dat raakt ons niet meer — deze PR neemt de oorzaak weg — maar het is wel iets waar andere gebruikers tegenaan lopen. Vandaar de vraag of we het melden, en waar. Wat er aan de hand isDe guard uit OpenAPITools/openapi-generator#23871 slaat aan zodra het gemeenschappelijke pad van een tag de routes van een ándere tag zou afvangen. Maar hij slaat juist níét aan wanneer twee tags exact hetzelfde pad delen — en dat is de zwaarste vorm van overlap. In for (String path : allResourcePaths) {
if (currentTagPaths.contains(path)) {
continue;
}
if (path.startsWith(commonPath + "/") || path.equals(commonPath)) {
return true;
}
}
Wat ik heb nagetrokken
Waarom dit een melding is en geen PRDe voor de hand liggende fix — die Te kiezen
Mijn voorkeur is 1. De tekst ligt klaar; hieronder, zodat wie hem plaatst niets hoeft te herschrijven. Concept-issue (Engels)Title: DescriptionThe shadowing guard added in #23871 (for #23414) does not fire when two tags declare operations on the same path. In that case one tag keeps the common prefix as its class-level The cause is the skip in for (String path : allResourcePaths) {
if (currentTagPaths.contains(path)) {
continue;
}
if (path.startsWith(commonPath + "/") || path.equals(commonPath)) {
return true;
}
}
Reproduction
openapi: 3.0.3
info:
title: Shadow repro
version: 1.0.0
tags:
- name: Alpha
- name: Beta
paths:
/items:
post:
tags: [Alpha]
operationId: createItem
responses:
'201':
description: created
get:
tags: [Beta]
operationId: listItems
responses:
'200':
description: ok
/items/{itemId}:
get:
tags: [Beta]
operationId: getItem
parameters:
- name: itemId
in: path
required: true
schema:
type: string
responses:
'200':
description: okGenerated with // BetaApi.java
@Path("/items")
public interface BetaApi {
@GET void listItems();
@GET @Path("/{itemId}") void getItem(@PathParam("itemId") String itemId);
}
// AlphaApi.java
@Path("")
public interface AlphaApi {
@POST @Path("/items") void createItem();
}Walking through the check: ImpactThe result depends on how the runtime matches requests. On Quarkus REST (RESTEasy Reactive) 3.38.1 resolution happens on the class-level base path first: Concretely, in our API three of six operations became unreachable after upgrading from 7.23.0 to 7.24.0: a NotesWe have resolved this on our side by giving every tag a disjoint path subtree, so we are not blocked. We are reporting it because the guard silently does not fire in a case it appears intended to cover, and the failure mode is a route that disappears without any build-time signal. One caveat on the obvious fix: simply removing the Question for maintainers: are two tags sharing an identical path meant to be supported by the jaxrs-spec generator? If it is, the shadowing check likely needs to account for it; if it is not, a generation-time warning would make the constraint visible instead of surfacing as a 405 at runtime. openapi-generator version7.24.0 (regression relative to 7.23.0, which emitted a class-level |
JaCoCo coverage
Files
|
7cbd040 to
287e875
Compare
Bumps org.openapitools:openapi-generator-maven-plugin from 7.23.0 to 7.24.0. --- updated-dependencies: - dependency-name: org.openapitools:openapi-generator-maven-plugin dependency-version: 7.24.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
openapi-generator 7.24.0 verdeelt paden anders over class- en methode-niveau: zodra een common prefix routes van een andere tag zou shadowen, valt de class-`@Path` weg en dragen de methodes het volledige pad (upstream PR #23871). De resource-classes zetten zelf `@Path(ApiInfo.BASE_PATH + "/berichten")`, dus dat leverde `/api/v1/berichten/berichten` op — POST /api/v1/berichten gaf 405. Een 405 wordt beantwoord zonder de request-body te lezen; de aanlever-test die 25 MiB uploadt bleef daardoor schrijven tot GitHub de job na 6 uur afkapte. De prefix staat nu één keer in `quarkus.rest.path`. Resources die een gegenereerde interface implementeren dragen geen eigen `@Path` meer en volgen de spec; handgeschreven resources houden een relatief pad. HAL-links in het magazijn laten `.path(ApiInfo.BASE_PATH)` weg omdat `uriInfo.baseUri` de prefix nu al bevat. De test-job krijgt `timeout-minutes: 30` als vangnet tegen een hangende upload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
De HAL-links in het magazijn laten `.path(ApiInfo.BASE_PATH)` weg sinds de prefix één keer via `quarkus.rest.path` gezet wordt: `uriInfo.baseUri` draagt hem dan al. De unit-test bouwde zijn base-URI echter zonder die prefix, waardoor de self-link-assertie op `/api/v1/berichten/...` niet meer kon slagen. De fixture bevat de prefix nu net als de runtime-waarde.
…één tag
De code-generator laat het class-basispad van een tag vervallen zodra dat pad de
routes van een andere tag zou afvangen; de operaties dragen dan hun volledige pad
op methodeniveau. Quarkus REST resolvet eerst op class-basispad en beantwoordt zo'n
operatie met 405. In het magazijn deelden Aanlever, Ophaal en Beheer allemaal
/berichten, waardoor aanleveren, status bijwerken en verwijderen onbereikbaar werden.
Elke tag bezit nu zijn eigen pad-subtree. Aanleveren verhuist naar
`POST /aanleveringen` — een eigen resource, symmetrisch met `POST /aanmeldingen`
in de uitvraag, en het houdt de afzender-kant gescheiden van de ontvanger-kant.
Ophaal en Beheer gaan samen onder één tag `Berichten`, want `GET` en
`PATCH`/`DELETE` op `/berichten/{berichtId}` delen hetzelfde pad en kunnen daarom
niet in verschillende tags zitten. De domeinlogica blijft wél gescheiden: alleen
de HTTP-laag komt samen in `BerichtenResource`, de services blijven los.
BREAKING CHANGE: aanleveren gebeurt op `POST /api/v1/aanleveringen` in plaats van
`POST /api/v1/berichten`. De lees- en beheerpaden onder `/berichten` blijven gelijk,
net als de `Location`-header en de HAL-links na een geslaagde aanlevering.
Aanleveren staat niet langer op `/berichten` maar op `/aanleveringen`, dus de spec telt vier paden in plaats van drie. Ook de verwijzing naar `ApiInfo.BASE_PATH` klopt niet meer: de prefix komt uit `quarkus.rest.path`.
287e875 to
afec46d
Compare
Fuzz-basis-image loopt achter op de dependency-declaratiesDe dependency-declaraties zijn gewijzigd, dus het fuzz-basis-image met de warme Blokkeert niets. De fuzz-controle werkt gewoon; hij haalt alleen het verschil Na de merge naar |
…dat meet Beide federatie-smokes leverden aan op `POST /api/v1/berichten`. Sinds #203 heeft aanleveren een eigen resource (`POST /api/v1/aanleveringen`) en bestaat op `/berichten` alleen nog `GET`, dus het magazijn antwoordde 405. De notificatie-smoke kon in deze samenstelling nooit groen zijn; `smoke-keten.sh` is daarmee al rood op main. Assert 3 van de notificatie-smoke wachtte `PUBLICATIE_INTERVAL` (5s) op een tweede aflevering, terwijl een retry alleen door de outbox-poller kan komen en die op 60s staat. De tweede aflevering viel per constructie buiten het venster: de assert was structureel groen, ook als het magazijn wél in de retry-lus zat. Het venster hangt nu aan een volle outbox-ronde. Verder: de gegenereerde test-BSN begint op 9 (RvIG-testbereik, dus nooit een uitgegeven nummer) en gaat via stdin de curl in in plaats van op de commandoregel, waar hij voor elke gebruiker op de machine in `ps` zichtbaar is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(plans): ontwerp en implementatieplan voor notificatie-events via FSC Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(common): maak het FSC-outway-headerpaar transport-onafhankelijk Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(magazijn): lever CloudEvents af door de eigen FSC-outway Een downstream met een grant-hash krijgt Fsc-Grant-Hash en Fsc-Transaction-Id mee en valt buiten de SSRF-blocklist: de outway kiest de bestemming op het contract achter die hash, niet op onze URL, en een outway-ClusterIP resolveert naar RFC1918. De TLS-eis blijft gelden; een actieve uitzondering logt bij boot DOWNSTREAM_VIA_OUTWAY. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(magazijn): maak de notificatie-downstream via de outway configureerbaar Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(plans): corrigeer servicenaam en compose-validatie in het notificatieplan Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(demo): geef magazijn-a een outway, zodat het uitgaand door FSC kan Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(demo): geef de outway van magazijn-a zijn eigen federatie-adres Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(demo): laat een inway meerdere diensten publiceren Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(demo): contract magazijn->notificatie naast het ophaalcontract Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(demo): smoke voor de notificatie-push door de FSC-keten Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(demo): lees de contractenlijst door tot de cursor leeg is De contract-bootstrap brak af zodra de manager een next_cursor meestuurde, in de aanname dat een cursor betekent dat de lijst is afgekapt. OpenFSC v2.5.2 zet die cursor op elke pagina die rijen bevat — ook als die pagina de hele lijst is en de opgevraagde limit ruim gehaald wordt; de volgende pagina komt leeg terug met een lege cursor. Daardoor werkte de bootstrap alleen op een volstrekt lege manager en faalde elke run daarna, terwijl hij op ZAD juist in een lus draait. fsc_contracten_paginas leest nu door tot de cursor leeg is en levert de samengevoegde lijst; de beoordeling verliest daarmee haar cursor-afbreking. De testvorm die de oude guard borgde gebruikte de topniveau-variant next_cursor, terwijl de manager 'm genest onder pagination zet — vandaar dat de suite groen bleef. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(magazijn): spreek de outway op HTTP/1.1 aan De JDK-client onderhandelt op een plain-http-doel eerst een upgrade naar h2c. De outway proxyt die hop-by-hop-headers ongewijzigd door naar de inway, waar het Go-http2-transport ze weigert met "invalid Upgrade request header"; de outway antwoordt dan 502 en het magazijn blijft herproberen. De FSC-data-plane is HTTP/1.1 — dat staat zo in het publicatiecontract van elke dienst. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(demo): leg de notificatie-push via FSC vast in runbooks en README Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: verwerk de reviewbevindingen op de notificatie-push - outway opgenomen in de csr-generator van magazijn-a, zodat een project-/ deploymentwissel ook dat certificaat bijwerkt in plaats van het op de oude SAN's te laten staan - de smoke telt de afleveringen opnieuw na een settle-venster; de stand uit de eerste waarneming kon een retry-stapeling per definitie niet zien - de nieuwe %dev-sleutel staat in de vangrail die env-var-expansie pint - de outway-aflevering logt haar Fsc-Transaction-Id, zodat een 502 te correleren is met de rij in beide txlogs - valideerUrl is internal en wordt rechtstreeks getoetst; de test deed een echte call naar een RFC1918-adres en gaf een andere uitkomst op een machine die dat adres wél kan bereiken - NOTIFICATIE_URL en NOTIFICATIE_GRANT_HASH staan in de env-var-tabel Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(demo): lever aan op /aanleveringen, en geef assert 3 een venster dat meet Beide federatie-smokes leverden aan op `POST /api/v1/berichten`. Sinds #203 heeft aanleveren een eigen resource (`POST /api/v1/aanleveringen`) en bestaat op `/berichten` alleen nog `GET`, dus het magazijn antwoordde 405. De notificatie-smoke kon in deze samenstelling nooit groen zijn; `smoke-keten.sh` is daarmee al rood op main. Assert 3 van de notificatie-smoke wachtte `PUBLICATIE_INTERVAL` (5s) op een tweede aflevering, terwijl een retry alleen door de outbox-poller kan komen en die op 60s staat. De tweede aflevering viel per constructie buiten het venster: de assert was structureel groen, ook als het magazijn wél in de retry-lus zat. Het venster hangt nu aan een volle outbox-ronde. Verder: de gegenereerde test-BSN begint op 9 (RvIG-testbereik, dus nooit een uitgegeven nummer) en gaat via stdin de curl in in plaats van op de commandoregel, waar hij voor elke gebruiker op de machine in `ps` zichtbaar is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: bind de SSRF-uitzondering aan de eigen outway-host De blocklist werd overgeslagen op grond van "er staat een grant-hash", terwijl de rechtvaardiging is dat het FSC-contract achter die hash de bestemming bepaalt. Die twee vallen alleen samen als de URL de outway ook echt aanwijst, en niets dwong dat af: één env-var opende daarmee de proxy-primitive die het comment bij `blokkeerIntern` zelf als dreiging benoemt. Nieuwe sleutel `magazijn.publicatie.outway.host` (env `OUTWAY_HOST`). De uitzondering geldt alleen bij een exacte host-match; ontbreekt de host of wijst de URL ergens anders heen, dan is het resultaat een terminale configuratiefout met die reden in plaats van stil verkeer. De TLS-eis blijft ervóór staan en verandert niet. Verder uit dezelfde reviewronde: - De boot-melding verhuist naar een `StartupEvent`-observer, zodat "logt bij boot" klopt; hij noemt nu de outway-host, en meldt apart welke downstreams een grant-hash dragen zonder dat hun URL erbij past. - Een grant-hash die niet in een HTTP-header past wordt vooraf afgekeurd. Zonder die controle gooide `header(...)` een IllegalArgumentException langs `lever()` heen: de claim kwam terug in TE_PUBLICEREN zonder dat `pogingen` opliep en struikelde elke ronde opnieuw. Alleen witruimte valt daar nu ook onder; leeg blijft "bewust geen outway". - De faalreden draagt een begrensd, gesaneerd fragment van de antwoordbody en de transaction-id, zodat een eigen configuratiefout te onderscheiden is van een fout van de bestemming en de rij in de txlogs terug te vinden is. - Tests op cardinaliteit (leeg/één/meerdere downstreams), op de configbinding van `grant-hash` en `outway.host`, op de boot-melding, en op de invariant dat de URL nooit gelogd wordt. De %prod-regels staan nu ook in ApplicationPropertiesTest. - `fsc_contracten_paginas` krijgt zes testvectoren; de guard die verdween had er geen. - De demo-stack hangt de grant-hash niet meer onvoorwaardelijk aan een URL die rechtstreeks naar toxiproxy gaat. - Motivering van de HTTP/1.1-pin en van `Fsc-Grant-Hash` naar de juiste bron: beide zijn OpenFSC-implementatie-eisen, niet fsc-core. - De runbook van magazijn-a beweerde dat de outway geen ingress-route nodig heeft. Dat klopt voor het mesh-verkeer maar niet voor de hop app naar outway, die de tenant-baseline-NetworkPolicy kruist. Plan: docs/plans/2026-08-20-review-notificatie-via-fsc.md Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(demo): lees de grant-hash uit fsc-grants.env met de helper die de-escapet De instructie in federatie/README.md schreef `set -a; . demo/generated/fsc-grants.env` voor. Dat bestand is een compose-`env_file` en draagt de dollars verdubbeld (`$$1$$3$$M_WDVC…`); sourcen in een shell expandeert `$$` naar het PID, dus de container kreeg `33956213395623339562M_WDVC…` en de outway antwoordde `400 UNKNOWN_GRANT_HASH`. De notificatie-smoke was daarmee rood op de asserts 1 tot en met 3, terwijl er niets mis was met de code. `fsc_compose_env_lees` bestaat precies hiervoor en waarschuwt er in zijn eigen comment voor; de smokes gebruiken hem al. Alleen de README omzeilde hem — en sinds `berichtenmagazijn-a` geen `env_file` meer heeft (waar compose het wél goed de-escapet) was dat de enige route naar een werkende push. De foutmelding van assert 1 wijst nu naar de magazijn-log, waar de outbox de werkelijke reden per poging rapporteert. Zonder die verwijzing meldt de smoke alleen dat er niets aankwam en moet je zelf raden waar je gaat kijken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 participants
Wat er verandert
De code-generator-update naar 7.24.0 maakte drie van de zes magazijn-operaties onbereikbaar.
Deze PR neemt de update alsnog, door de oorzaak weg te nemen in plaats van de update tegen te
houden.
De oorzaak. Upstream "Fix path shadowing"
laat het class-basispad van een tag vervallen zodra dat pad de routes van een ándere tag zou
afvangen; de operaties dragen hun volledige pad dan op methodeniveau. Dat is geldige JAX-RS, maar
Quarkus REST resolvet eerst op class-basispad en beantwoordt zo'n operatie met 405. In het magazijn
deelden
Aanlever,OphaalenBeheerallemaal/berichten:@PathOphaalApi"/berichten"AanleverApi""POST /berichten→ 405BeheerApi""PATCH/DELETE /berichten/{berichtId}→ 405De oplossing. Elke tag bezit nu zijn eigen pad-subtree:
POST /berichtenPOST /aanleveringenAanlever/berichten…Ophaal→Berichten/berichten/{berichtId}Beheer→BerichtenGegenereerd wordt nu
AanleverApi @Path("/aanleveringen")naastBerichtenApi @Path("/berichten")—twee disjuncte subtrees, geen lege class-
@Pathmeer.OphaalenBeheermoesten één tag worden omdatGETenPATCH/DELETEop/berichten/{berichtId}hetzelfde pad delen; geen ADR-conforme hernoeming scheidt die. Alleen deHTTP-laag komt daardoor samen in
BerichtenResource—BerichtOphaalServiceenBerichtBeheerServiceblijven gescheiden, net als hun tests.Aanleveren houdt wél een eigen resource. Het is de afzender-kant die via de FSC-inway naar binnen
schrijft, tegenover de ontvanger-kant met
X-OntvangerenBerichtAutorisatie.vereisOntvanger(...).Die twee autorisatieregimes in één class zetten zou de verkeerde besparing zijn.
Waarom niet wachten
De bijbehorende Quarkus-issue (quarkusio/quarkus#26496)
is gesloten en gerepareerd in 3.24.0.CR1, ruim vóór onze 3.38.1. Die fix dekt
@Path("/base")tegenover
@Path("/base/{id}"), niet@Path("")tegenover@Path("/berichten")— gemeten: de 405'streden op onze huidige Quarkus-versie nog steeds op. De generator doet niets fouts, dus wachten
lost hier niets op.
Wel opgevallen en apart te melden bij OpenAPITools: de shadow-detectie is asymmetrisch.
Ophaal's commonPath/berichtenwas exact gelijk aanAanlever's pad, dus had ookOphaalzijnclass-
@Pathmoeten verliezen. Dan hadden alle drie de interfaces@Path("")gehad en had deroutering gewoon gewerkt.
Meegenomen fout
BerichtDtoMapperTestfaalde op deze branch, los van de bump. De HAL-links laten.path(ApiInfo.BASE_PATH)weg sinds de prefix één keer viaquarkus.rest.pathgezet wordt, maar detest-fixture bouwde zijn base-URI nog zonder die prefix. De fixture spiegelt nu de runtime-waarde.
Wat er meeverhuist
Alleen het schrijfpad.
MagazijnAanleverClientin de demo-console (class-@Pathnaar/api/v1,paden per methode, want POST en PATCH zitten nu op verschillende roots), de Bruno-map
berichten/→aanleveren/,demo/smoke.sh,apis.jsonen 34 POST-call-sites in tests. Delees-clients —
MagazijnClientin de sessiecache-library én in de uitvraag — raken/berichtenenblijven ongemoeid, net als de WireMock-mappings (die stubben leespaden).
Verificatie
./mvnw clean verify— alle modules groen: fbs-common 296, magazijnregister 30, sessiecache 295,magazijn 379, uitvraag 194, demo-console 43
RouteDekkingTestuit test(api): bewaak dat elk pad uit de spec ook echt bij een resource aankomt #192 tijdelijk meegedraaid op deze branch: 12/12 groen, waar dezelfdetest vóór deze wijziging drie 405's meldde
Vervangt #147
Deze wijziging zat eerst op de dependabot-branch van #147. De deploy-workflow slaat bouwen en
preview-deployen bewust over voor bot-PR's, waardoor de drie
deploy-preview-*-checks daar opskippedbleven staan — bij een brekende contractwijziging is dat precies de verificatie die jeniet wilt missen. Dezelfde commits staan hier op een eigen branch, zodat de volledige CI draait.
De bump naar 7.24.0 reist mee; #147 kan dicht.
Gestapeld op #192
Deze PR staat op de branch van #192 en moet er dus ná mergen. Dat is geen volgorde-afspraak maar
een echte afhankelijkheid: #192 legt in
RouteDekkingTestvast dat de magazijn-spec drie padentelt, en deze PR maakt er vier van. Los gemerged zou de bewaker op
mainomvallen. Gestapelddraait die test hier meteen tegen de nieuwe spec — 12/12 groen, waar dezelfde test vóór deze
wijziging drie 405's meldde.
De diff hierboven toont alleen het eigen werk; GitHub zet de base op
mainzodra #192 gemerged is.Closes MinBZK/MijnOverheidZakelijk#870
🤖 Generated with Claude Code